feat: improve builder by ascandone · Pull Request #165 · formancehq/numscript

ascandone · 2026-06-23T15:07:48Z

No description provided.

coderabbitai · 2026-06-23T15:08:03Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c6e91a66-d0bc-41e0-b0a7-831e48790a40

📥 Commits

Reviewing files that changed from the base of the PR and between 08b4640 and 17a2a11.

📒 Files selected for processing (1)

builder/builder.go

🚧 Files skipped from review as they are similar to previous changes (1)

builder/builder.go

Walkthrough

Adds BuildProgramWithFeatureFlags to builder/builder.go: a helper checkIsFlagValid enforces that flag strings contain only lowercase letters and -, and the exported function prepends a #![feature(...)] header to the BuildProgram output. A new test TestSimpleSendWithFlag validates the generated script with two feature flags.

Changes

Feature Flag Builder

Layer / File(s)	Summary
Flag validation and `BuildProgramWithFeatureFlags` implementation `builder/builder.go`	Removes a stale TODO comment. Adds `checkIsFlagValid` (returns false for empty or non-`[a-z-]` strings) and exported `BuildProgramWithFeatureFlags`, which panics on invalid flags, formats the comma-separated feature list, and prepends `#![feature(...)]` to the script returned by `BuildProgram`.
`TestSimpleSendWithFlag` test `builder/builder_test.go`	Adds a test that builds a `StmtSend` via `BuildProgramWithFeatureFlags` with `my-flag` and `another-flag`, and asserts the output script against an inline snapshot containing the feature header and expected `vars`/`send` body.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 Hippity-hop, a flag goes in,
With lowercase letters and dashes within.
#![feature] sits atop the script,
Invalid names get promptly clipped!
The rabbit stamps the snapshot — approved! ✅

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is related to the builder changes but too generic to convey the main update clearly.	Use a more specific title that mentions feature-flag support or program header generation in the builder.
Description check	❓ Inconclusive	No pull request description was provided, so there is nothing to assess beyond a missing summary.	Add a brief description of the builder feature-flag support and the new test coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/improve-builder

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-06-23T15:09:27Z

Codecov Report

❌ Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.48%. Comparing base (9ca635b) to head (17a2a11).

Files with missing lines	Patch %	Lines
builder/builder.go	71.42%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #165      +/-   ##
==========================================
- Coverage   68.52%   68.48%   -0.05%     
==========================================
  Files          56       56              
  Lines        5417     5438      +21     
==========================================
+ Hits         3712     3724      +12     
- Misses       1494     1498       +4     
- Partials      211      216       +5

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

NumaryBot

✅ Approve — automated review

The new builder API emits syntactically valid feature declarations and preserves existing BuildProgram behavior. I did not find any discrete regressions introduced by the patch.

No findings.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

builder/builder_test.go (1)

53-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a dedicated test for invalid-flag panic behavior.

This PR introduces a strict panic contract for invalid flags, but current coverage only asserts the happy path. Add a focused test to lock panic behavior and prevent silent contract drift.

Example test addition

+func TestBuildProgramWithFeatureFlagsInvalidFlagPanics(t *testing.T) {
+	stmt := builder.StmtSend(
+		builder.ExprMonetary(
+			builder.ExprAsset("USD/2"),
+			builder.ExprNumberBigInt(big.NewInt(1)),
+		),
+		builder.SrcAccount(builder.ExprAccount("src")),
+		builder.DestAccount(builder.ExprAccount("dest")),
+	)
+
+	defer func() {
+		if recover() == nil {
+			t.Fatalf("expected panic for invalid feature flag")
+		}
+	}()
+
+	builder.BuildProgramWithFeatureFlags([]string{"Invalid-Flag"}, stmt)
+}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@builder/builder_test.go` around lines 53 - 68, The current test coverage only
validates the happy path where valid feature flags are passed to the
BuildProgramWithFeatureFlags function. Add a new test function that specifically
tests the panic behavior when an invalid or unsupported flag is provided to
BuildProgramWithFeatureFlags. The test should verify that the function panics
with an appropriate error message when given an invalid flag, ensuring the
strict panic contract is properly covered and preventing regression of the panic
behavior contract.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@builder/builder.go`:
- Around line 175-189: The BuildProgramWithFeatureFlags function validates
featureFlags after calling BuildProgram on the statements, which allows side
effects from BuildProgram to execute even when invalid flags would cause a
panic. Move the flag validation loop that calls checkIsFlagValid before the call
to BuildProgram to ensure invalid flags are caught immediately and prevent any
statement execution or side effects from occurring before the panic.

---

Nitpick comments:
In `@builder/builder_test.go`:
- Around line 53-68: The current test coverage only validates the happy path
where valid feature flags are passed to the BuildProgramWithFeatureFlags
function. Add a new test function that specifically tests the panic behavior
when an invalid or unsupported flag is provided to BuildProgramWithFeatureFlags.
The test should verify that the function panics with an appropriate error
message when given an invalid flag, ensuring the strict panic contract is
properly covered and preventing regression of the panic behavior contract.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0137bf64-5872-438a-8436-36decab91be4

📥 Commits

Reviewing files that changed from the base of the PR and between 9ca635b and 08b4640.

📒 Files selected for processing (2)

builder/builder.go
builder/builder_test.go

paul-nicolas

LGTM

NumaryBot

✅ Approve — automated review

The new builder helper appears to generate syntactically valid feature declarations for the supported flag format, and existing builder behavior is unchanged.

No findings.

ascandone added 2 commits June 23, 2026 16:56

chore: removed todo comment

95c0043

feat: add flags

08b4640

ascandone requested a review from paul-nicolas June 23, 2026 15:08

NumaryBot reviewed Jun 23, 2026

View reviewed changes

coderabbitai Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread builder/builder.go

paul-nicolas previously approved these changes Jun 26, 2026

View reviewed changes

ascandone enabled auto-merge (squash) June 26, 2026 07:59

fix: fix dirty build

17a2a11

ascandone dismissed paul-nicolas’s stale review via 17a2a11 June 26, 2026 09:09

NumaryBot reviewed Jun 26, 2026

View reviewed changes

ascandone requested a review from paul-nicolas June 26, 2026 09:12

paul-nicolas approved these changes Jun 26, 2026

View reviewed changes

ascandone merged commit 22135a6 into main Jun 26, 2026
6 of 7 checks passed

ascandone deleted the feat/improve-builder branch June 26, 2026 09:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: improve builder#165

feat: improve builder#165
ascandone merged 3 commits into
mainfrom
feat/improve-builder

ascandone commented Jun 23, 2026

Uh oh!

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading

❌ Failed checks (1 warning, 2 inconclusive)

Uh oh!

codecov Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

NumaryBot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

paul-nicolas left a comment

Uh oh!

NumaryBot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ascandone commented Jun 23, 2026

Uh oh!

coderabbitai Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning, 2 inconclusive)

Uh oh!

codecov Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NumaryBot left a comment

Choose a reason for hiding this comment

✅ Approve — automated review

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paul-nicolas left a comment

Choose a reason for hiding this comment

Uh oh!

NumaryBot left a comment

Choose a reason for hiding this comment

✅ Approve — automated review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading

codecov Bot commented Jun 23, 2026 •

edited

Loading